-
Notifications
You must be signed in to change notification settings - Fork 35
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Only delete compcomm on icomm if it exists #715
Conversation
Within the MPI implementation, an inner communicator can optionally contain a compilation commucator. An indentation error meant that the compilation attribute was unconditionally deleted, causing incorrect MPI behaviour if the attribute had never been set in the first place. On MPICH, this doesn't seem to raise any errors, but with OpenMPI it causes errors like: mpi4py.MPI.Exception: MPI_ERR_OTHER: known error not in list
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, this looks like it was a mistake. Nice catch!
I will wait for the result of firedrakeproject/firedrake#3388 before merging, just to make sure nothing breaks.
I think there's something else hiding out under here. All the tests pass, but several of them try to clean up the null communicator (you can also see that on the master tests, so it wasn't introduced by this PR). I don't know what causes it, but one single-threaded example I found in my Docker container is
This shows the strange |
Okay, I will try and give some context for what I believe to be going on here: This diagram/essay helps a bit: https://github.com/OP2/PyOP2/blob/master/pyop2/mpi.py#L85-L160 When a user creates a comm(unicator) and uses it to create a Firedrake object it is duplicated by the PyOP2 layer to avoid communication clash (a similar model to what PETSc does). The user is responsible for this comm and the responsible user will clean that comm before the process terminates. The issue that causes these warning can stem from a user not cleaning the comm and having Python clean it up for them, which interferes with PyOP2's clean up process. One option here is to ignore The mysterious I will investigate the specific test case you have found, there is possibly an error somewhere in the test. |
Closing as this change has already been merged separately. |
Within the MPI implementation, an inner communicator can optionally contain a compilation commucator. An indentation error meant that the compilation attribute was unconditionally deleted, causing incorrect MPI behaviour if the attribute had never been set in the first place. On MPICH, this doesn't seem to raise any errors, but with OpenMPI it causes errors like: